Batch 4C pt1: HostConfig renderer fields + levels render-units (systemd cutover drop-ins) - #193

Update — increment 2 pushed (1c608f8): the systemd-unit renderer.

Since you approved the levels render-* mechanism + D-CONSUMER, I folded the first renderer into this PR (the HostConfig fields + their first consumer is one coherent unit). The PR now covers:

1. HostConfig renderer fields (service_user, service_home, release_root, fpm_pool_php) — unchanged from before.
2. levels render-units — emits one <unit>.service.d/cutover.conf per engine consumer (the 6 units that run levels from the venv), re-pointing each at /opt/kayak/current/venv + overriding the moved paths. Pure HostConfig→text (kayak.host_render), per the emit-config precedent; systemd-analyze verify happens on the VM.

A real finding from building it: fetch-osmb and audit-gauges default their generated-data dirs (map-layer staging; the gauge-metadata cache) relative to the install root (config.py BASE_DIR), which is read-only under /opt/kayak/current. So their drop-ins must relocate those to /var/cache/kayak/*. Two new keep-current-then-flip fields carry it (map_layers_dir, gauge_metadata_cache) — defaults are the live repo-relative paths; the cutover host.yaml flips them, exactly like docroot.

Each drop-in resets ExecStart (empty ExecStart= then the new one — else a oneshot runs both) and resets ReadWritePaths (so the writable set is exactly the cutover reality), pins DATASET_DIR to the release's dataset snapshot (not the live kayak_data clone), and sets WorkingDirectory=/opt/kayak/current. Only pipeline sets OUTPUT_DIR (the #3 docroot); status passes its --output arg.

Tests: +15 (per-unit drop-in assertions + the CLI write/manifest/error paths). Full non-slow suite 1744 passed, ruff/format/mypy clean.

Next (separate PR): increment 3 = render-nginx / render-fpm (the vhost root + open_basedir substitutions + the server_names type).

Adversarial review — PR #193 (Batch 4C pt1: HostConfig renderer fields + levels render-units)

Reviewed on the live WKCC host, cross-checking the rendered drop-ins against the actual installed units, scripts, and paths. Static review (I didn't run the suite — author reports 1744 passed); I independently re-verified the consumer enumeration and the ExecStart/RWP parity against this host. Solid foundation PR — behavior-neutral and the hard part is right. One accuracy issue in the plan's "verified" classification, plus a defense-in-depth asymmetry and a forward-looking caveat.

What's right (the load-bearing parts)

• The 6-engine-consumer set is correct and complete. I enumerated every kayak-*.service on main: exactly six run levels from the venv — pipeline, decimate, editor-retention, fetch-osmb, status, audit-gauges — matching render-units precisely. The rest are shell scripts (healthcheck/config-drift/cert-expiry/recap/heartbeat) or host-level (backups/notify/fail-test/cert-renewal). The #191 audit-gauges→6th observation checks out.

• The regenerated ExecStart + ReadWritePaths reproduce each base unit exactly under the keep-current defaults — verified directive-by-directive against the live units:

  unit	base ReadWritePaths	rendered (default)	base args	rendered args
  pipeline	public_html DB	{docroot} DB	pipeline	pipeline ✓
  decimate	DB	DB	decimate	decimate ✓
  editor-retention	DB	DB	editor-retention	✓
  fetch-osmb	kayak/var/osmb	{map_layers_dir}	fetch-osmb	✓
  status	/home/pat/var	parent(status_output)	status --output …/status.html	✓
  audit-gauges	DB Gauge-metadata-cache	DB parent(cache)	audit-gauges --days 16 --email ${AUDIT_EMAIL}	✓

  The ReadWritePaths=/ExecStart= empty-reset is the correct drop-in idiom, the writable set flips coherently with its Environment= counterpart (docroot↔OUTPUT_DIR, map_layers↔MAP_LAYERS_DIR, cache↔GAUGE_METADATA_CACHE), and ${AUDIT_EMAIL}/--days 16/status_output are reproduced verbatim. That's the part most likely to silently break a sandbox at cutover, and it's correct.

• service_user's validator genuinely blocks User=/shell-ACL injection; the CLI mirrors emit-config and a malformed host.yaml returns 1 (confirmed it propagates through main.py's sys.exit(rc)).

1. (Medium) The plan's "verified against systemd/" classification mis-describes two class-B units

§20's table is the source of truth the eventual KAYAK_UNITS/KAYAK_HOST_UNITS derivation (open decision D-CONSUMER) will consume, so its accuracy matters. Two rows are wrong against the live scripts:

• kayak-recap — "run *.sh, NOT the venv" is false. systemd/kayak-recap.sh runs:

  "${KAYAK_HOME}/.venv/bin/python3" "${KAYAK_HOME}/kayak/scripts/recap.py" --days … --unit 'kayak-*'
  

  It does use the venv (the editable-install one). The "reads journald, not the DB" half is correct — recap.py shells to journalctl, no DB. Net: the disposition (don't re-point) is low-impact since post-cutover it just renders a journald email from the old /home/pat/.venv tree — but it contradicts §Goal ("every levels-running consumer must point at current"), and the venv mis-statement is exactly what "verified against systemd/" should have caught.
• kayak-heartbeat — "curl heartbeat, not the DB" is wrong on both counts. systemd/kayak-heartbeat.sh emails via mail/msmtp (no curl) and stats ${KAYAK_HOME}/DB/kayak.db (file mtime — not a content read). The disposition (no quiesce) still holds (a stat doesn't lock or read rows), but the description is inaccurate.

Recommend correcting the table so D-CONSUMER's list derivation rests on accurate facts: recap = "venv-python on a checkout script; journald only (no DB) → keep on checkout, no quiesce"; heartbeat = "mail heartbeat; stat()s the DB file (no content read) → no quiesce". Functionally the dispositions look right; the rationale just needs to match reality.

2. (Low) Path fields land in the same generated unit files as service_user, but skip its injection hardening

render_cutover_dropins interpolates the path fields by plain f-string — Environment=K={path}, ReadWritePaths={' '.join(paths)}, WorkingDirectory={path}, ExecStart={venv} … — yet _abs_path only checks startswith("/"). You deliberately tightened service_user because it's "interpolated into unit User= and shell ACL commands"; the paths land in those same files via the same mechanism with no such guard. A newline in a path field injects an arbitrary systemd directive into the drop-in (e.g. a second ExecStartPre=); a space splits a ReadWritePaths= entry. host.yaml is root-owned/trusted, so this is defense-in-depth rather than a live hole — but close the asymmetry by rejecting control chars/whitespace in the path validator. Two sub-nits while there: ^…$ lets "pat\n" pass _USERNAME_RE (Python $ matches before a trailing newline — use \A…\Z/fullmatch), and \d in _PHP_VERSION_RE matches Unicode digits (use [0-9]).

3. (Low, forward-looking) Rendered drop-ins are install-unsafe pre-cutover

Every drop-in references /opt/kayak/current (DATASET_DIR, WorkingDirectory, the ExecStart venv), which only exists after the first activation. Correct by design — render-units emits text and the cutover applies it — but a stray render-units --out-dir /etc/systemd/system on the pre-cutover host would point all six consumers' DATASET_DIR at a non-existent dir and break them. Worth a guard/warning when --out-dir targets a live systemd dir, or just a runbook note. (Also: --out-dir writes cutover.conf but never sweeps a stale one if the consumer set later shrinks — minor.)

Merge mechanics

Branch is 3 commits behind main and depends on #192's /var/cache/kayak/docroot (now merged) — that's the BLOCKED/not-up-to-date state. Rebase before merge; nothing in the diff conflicts.

Verdict

Approve in spirit — schema + renderer are clean, behavior-neutral, and the consumer/RWP/ExecStart parity (the easy-to-get-wrong part) is correct. Before this feeds the actual cutover, fix the recap/heartbeat rows in the classification table (#1), since that table is what the gate-list derivation will be built from.

Thanks — sharp review. All three addressed in 48b92b1, and the branch is now rebased onto merged #192 (the BLOCKED/not-up-to-date you flagged is resolved — 0 behind, clean replay since the diffs are disjoint).

1 (Medium) — the misclassified rows. You're right; I verified against the live scripts and corrected the plan:

• kayak-recap → runs ${KAYAK_HOME}/.venv/bin/python3 scripts/recap.py (the editable-install venv, not levels), journald only, no DB. Disposition unchanged (stays on the checkout — running old code to render a journald email is harmless; no quiesce). Added an explicit note that it's not class-A: it doesn't invoke levels and touches neither the migrated DB nor the release dataset, so §Goal doesn't reach it — and the D-CONSUMER derivation keys re-pointing off "ExecStart runs …/levels", which correctly excludes it.
• kayak-heartbeat → mail/msmtp (no curl); stat()s the DB file mtime (no content read, no lock) → no quiesce.

2 (Low) — the injection asymmetry. Closed it: _abs_path now rejects whitespace/control chars (a newline injects a directive, a space splits a ReadWritePaths= entry). Both sub-nits fixed too — ^…$ → \A…\Z on all four validators (so "pat\n" no longer slides through service_user/cert_host/offsite_remote), and \d → [0-9] in the PHP-version regex.

3 (Low) — install-unsafe pre-cutover. render-units --out-dir now warns when {release_root}/current doesn't exist (the drop-ins reference it, so applying them before the first activation breaks the six consumers). The stale-cutover.conf-sweep gap is noted inline as a (currently moot, fixed 6-unit set) follow-up.

Tests: +5 (trailing-newline username, Unicode-digit version, whitespace path, the warning present/absent). Full non-slow suite 1749 passed; ruff/format/mypy clean.